Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: c509 wasm binding js #628

Merged
merged 45 commits into from
Jul 30, 2024
Merged

feat: c509 wasm binding js #628

merged 45 commits into from
Jul 30, 2024

Conversation

bkioshn
Copy link
Contributor

@bkioshn bkioshn commented Jul 19, 2024 •
edited
Loading

Description

Implement WASM binding JS for C509 lib

Related Issue(s)

Closes #617

Description of Changes

  • Create a wrapper for wasm to be used in JS including
    • generate, verify, and decode function
    • string conversion to PrivateKey and PublicKey function
  • Create a sample on how to use all the function mention above in Javascript and test it on a basic .html file
  • Update Earthfile to generate the WASM package

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

bkioshn and others added 20 commits July 17, 2024 19:51
@bkioshn bkioshn marked this pull request as draft July 19, 2024 07:59
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 19, 2024 •
edited
Loading

✅ Test Report | ${\color{lightgreen}Pass: 105/105}$ | ${\color{red}Fail: 0/105}$ |

@bkioshn bkioshn changed the title Feat/c509 wasm bind js feat: c509 wasm binding js Jul 19, 2024
@bkioshn bkioshn self-assigned this Jul 19, 2024
@bkioshn bkioshn added the documentation Pull requests that update a dependency file. label Jul 19, 2024
@dtscalac
Copy link
Contributor

Had a look from a flutter web point of view. The created APIs can easily be called from dart via js_interop. Additionally with rather little effort we will be able to use the structures in Flutter that are returned by the wasm library. For flutter web this is a solid solution.

Looking a bit forward, wasm bindings are unusable on mobile/desktop platforms. Then I imagine we can rely on solutions like https://github.com/fzyzcjy/flutter_rust_bridge but from my experience it requires the rust code to be adapted to work with this bridge.

Base automatically changed from feat/c509_cli to main July 26, 2024 13:34
@bkioshn bkioshn marked this pull request as ready for review July 29, 2024 06:06
Mr-Leshiy
Mr-Leshiy reviewed Jul 29, 2024
View reviewed changes
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think to transform your simple html/js example into the node tests and run them as integration tests in earthly.
Something similar we already done (but without earthly) by adding integration tests running wasm code (https://github.com/input-output-hk/catalyst-core/tree/main/src/chain-wallet-libs/bindings/wallet-wasm-js/js-test).
Of course it will take some additional effort to do that, but seems to me it worth to do.
Maybe not as a part of this PR.

Mr-Leshiy
Mr-Leshiy approved these changes Jul 29, 2024
View reviewed changes
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bkioshn bkioshn mentioned this pull request Jul 30, 2024
Closed
@bkioshn bkioshn merged commit f913f0c into main Jul 30, 2024
34 checks passed
@bkioshn bkioshn deleted the feat/c509-wasm-bind-js branch July 30, 2024 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtscalac dtscalac dtscalac left review comments

@apskhem apskhem apskhem approved these changes

@Mr-Leshiy Mr-Leshiy Mr-Leshiy approved these changes

Assignees

@bkioshn bkioshn

Labels
documentation Pull requests that update a dependency file. enhancement New feature or request
Projects
Catalyst
Archived in project
Milestone
M3: Backend and Wallet Integration Up...
Development

Successfully merging this pull request may close these issues.

🛠️ [TASK] : Implement WASM binding for C509 lib
4 participants
@bkioshn @dtscalac @Mr-Leshiy @apskhem